Fix size of arrays in MillePedeDQMModule to store new HG thresholds by dmeuser · Pull Request #38367 · cms-sw/cmssw

dmeuser · 2022-06-14T13:25:06Z

resolves #38364

PR description:

Adressing issue #38364, by adjusting the array size of the threshold containers in MillePedeDQMModule to hold the new HG thresholds introduced in PR #38273 and #38195.

cmsbuild · 2022-06-14T13:31:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38367/30553

This PR adds an extra 16KB to repository

cmsbuild · 2022-06-14T13:32:15Z

A new Pull Request was created by @dmeuser for master.

It involves the following packages:

Alignment/MillePedeAlignmentAlgorithm (alca)

@cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks.
@mmusich, @adewit, @tocheng, @tlampen this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

mmusich · 2022-06-14T13:33:30Z

please test

mmusich · 2022-06-14T13:34:08Z

resolves #38364

mmusich · 2022-06-14T13:36:00Z

Alignment/MillePedeAlignmentAlgorithm/plugins/MillePedeDQMModule.h

-                       const std::array<double, 6>& sigCut,
-                       const std::array<double, 6>& maxMoveCut,
-                       const std::array<double, 6>& maxErrorCut,
+                       const std::array<double, 8>& cut,


as a suggestion, since this number doesn't need to be different for the various arrays, could it be a named constant defined somewhere and then used everywhere?

Sure, do you want me to change it in this PR or the in the upcoming one?

maybe could be done directly here.

Yes, please, do it here.
As such you can also better identify if anything is missing the update (for example: shouldn't also the dimensions and number of bins of the histos get modified accordingly?)

Same argument as below(#38367 (comment)). We want to keep the new HG alignment separated from the LG one, thus we will add additional histograms for the HG result in an upcoming PR. Most of the changes were already collected here: https://github.com/dmeuser/cmssw/tree/hgPCL_PR, where we still need some adaptions to make the next PR

perrotta · 2022-06-14T13:41:34Z

Alignment/MillePedeAlignmentAlgorithm/plugins/MillePedeDQMModule.cc

+                                          const std::array<double, 8>& sigCut,
+                                          const std::array<double, 8>& maxMoveCut,
+                                          const std::array<double, 8>& maxErrorCut,
                                          const std::array<double, 6>& obs,


Why still "6"?

Because the fillExpertHisto method will only take care of the low granularity result, which has 6 different structures. We will add a new method in an upcoming PR to take care of the high granularity result.

cmsbuild · 2022-06-14T14:07:14Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38367/30556

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38367/30556/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38367/30556/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2022-06-14T14:25:32Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38367/30558

This PR adds an extra 16KB to repository

cmsbuild · 2022-06-14T14:25:57Z

Pull request #38367 was updated. @cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please check and sign again.

tvami · 2022-06-14T18:28:31Z

@cmsbuild , please test for CMSSW_12_5_CLANG_X

tvami · 2022-06-14T18:29:01Z

@cmsbuild , please test

let's see the vanilla one too

cmsbuild · 2022-06-14T22:12:20Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a85fdb/25526/summary.html
COMMIT: 6b7b2f6
CMSSW: CMSSW_12_5_X_2022-06-14-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38367/25526/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3659074
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3659044
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 49 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-06-14T23:05:07Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a85fdb/25525/summary.html
COMMIT: 6b7b2f6
CMSSW: CMSSW_12_5_CLANG_X_2022-06-13-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38367/25525/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 77931 lines to the logs
Reco comparison results: 23260 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3659074
DQMHistoTests: Total failures: 108774
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3550277
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: found differences in 1 / 49 workflows

perrotta · 2022-06-15T04:59:43Z

@cms-sw/alca-l2 I think you can sign this, since you already did so for its 12_4_X backport #38371

tvami · 2022-06-15T05:33:12Z

+1

cmsbuild · 2022-06-15T05:33:34Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2022-06-15T07:03:06Z

+1

Fix size of arrays to store new HG thresholds

c597a1d

cmsbuild added this to the CMSSW_12_5_X milestone Jun 14, 2022

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures tests-pending labels Jun 14, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 14, 2022

cmsbuild added tests-started and removed tests-pending labels Jun 14, 2022

mmusich reviewed Jun 14, 2022

View reviewed changes

perrotta reviewed Jun 14, 2022

View reviewed changes

cmsbuild added code-checks-pending tests-pending and removed tests-started code-checks-approved labels Jun 14, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jun 14, 2022

Store length of threshold and LG result array in enums

6b7b2f6

dmeuser force-pushed the bugFix_HGthresholds_clang branch from fbf3056 to 6b7b2f6 Compare June 14, 2022 14:17

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jun 14, 2022

cmsbuild removed the code-checks-pending label Jun 14, 2022

cmsbuild added the code-checks-approved label Jun 14, 2022

mmusich mentioned this pull request Jun 14, 2022

New PCL Alignment Threshold object and record for HG PCL Alignment [12.4.X] #38371

Merged

cmsbuild added tests-started and removed tests-pending labels Jun 14, 2022

cmsbuild added tests-approved and removed tests-started labels Jun 14, 2022

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Jun 15, 2022

cmsbuild added orp-approved and removed orp-pending labels Jun 15, 2022

cmsbuild merged commit 8f90e26 into cms-sw:master Jun 15, 2022

cmsbuild mentioned this pull request Jun 15, 2022

[L1T] Phase-2 update phase 2 emulator sequence #38375

Closed

perrotta mentioned this pull request Jun 15, 2022

[CLANG_X] Segmentation violation in MillePedeDQMModule::dqmEndJob #38364

Closed

cmsbuild mentioned this pull request Jun 15, 2022

[DEVEL] Added alpaka build rules cms-sw/cmsdist#7936

Merged

dmeuser mentioned this pull request Jun 21, 2022

New high granularity pixel alignment for the PCL #38449

Merged

Conversation

dmeuser commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

Uh oh!

cmsbuild commented Jun 14, 2022

Uh oh!

cmsbuild commented Jun 14, 2022

Uh oh!

mmusich commented Jun 14, 2022

Uh oh!

mmusich commented Jun 14, 2022

Uh oh!

mmusich Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

dmeuser Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

mmusich Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

perrotta Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

dmeuser Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

perrotta Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

dmeuser Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Jun 14, 2022

Uh oh!

cmsbuild commented Jun 14, 2022

Uh oh!

cmsbuild commented Jun 14, 2022

Uh oh!

tvami commented Jun 14, 2022

Uh oh!

tvami commented Jun 14, 2022

Uh oh!

cmsbuild commented Jun 14, 2022

Comparison Summary

Uh oh!

cmsbuild commented Jun 14, 2022

Comparison Summary

Uh oh!

perrotta commented Jun 15, 2022

Uh oh!

tvami commented Jun 15, 2022

Uh oh!

cmsbuild commented Jun 15, 2022

Uh oh!

perrotta commented Jun 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dmeuser commented Jun 14, 2022 •

edited

Loading